Skip to content

Closures inherit #[optimize] from the enclosing function by default.#158901

Open
veluca93 wants to merge 1 commit into
rust-lang:mainfrom
veluca93:optimize-attr-closure-inherit
Open

Closures inherit #[optimize] from the enclosing function by default.#158901
veluca93 wants to merge 1 commit into
rust-lang:mainfrom
veluca93:optimize-attr-closure-inherit

Conversation

@veluca93

@veluca93 veluca93 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: #54882
Stabilization PR: #157273

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 7, 2026
@rustbot

rustbot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 19 candidates

@veluca93 veluca93 mentioned this pull request Jul 7, 2026
Open
@fee1-dead fee1-dead added S-waiting-on-t-lang Status: Awaiting decision from T-lang and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2026
@fee1-dead

fee1-dead commented Jul 9, 2026

Copy link
Copy Markdown
Member

Waiting on t-lang per #157273 (comment)

@fee1-dead fee1-dead added I-lang-nominated Nominated for discussion during a lang team meeting. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels Jul 9, 2026
@tmandry

tmandry commented Jul 16, 2026

Copy link
Copy Markdown
Member

The lang team discussed this change today and was in favor: #157562 (comment)

@veluca93

veluca93 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

I don't believe this is blocked on lang anymore, but I don't think I can remove the tag myself.

@mejrs mejrs added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-t-lang Status: Awaiting decision from T-lang labels Jul 21, 2026
fee1-dead
fee1-dead requested changes Jul 22, 2026
View reviewed changes

@fee1-dead fee1-dead left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r? scottmcm
could you check if the codegen-llvm CHECK annotations here are idiomatic or is there a better way?

View changes since this review

Comment thread tests/codegen-llvm/optimize-closures-inheritance.rs
@rustbot rustbot assigned scottmcm and unassigned fee1-dead Jul 22, 2026
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 22, 2026
@rustbot

rustbot commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@veluca93
veluca93 force-pushed the optimize-attr-closure-inherit branch from 3c5ebd5 to f24fa6f Compare July 22, 2026 05:49
@veluca93

veluca93 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2026
@wesleywiser

wesleywiser commented Jul 23, 2026

Copy link
Copy Markdown
Member

FileCheck tests look fine to me but maybe @scottmcm knows better 🙂

@bors rollup=never

Could have perf implications from changing effective optimization values for closures.

scottmcm
scottmcm requested changes Jul 24, 2026
View reviewed changes

@scottmcm scottmcm left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cg_ssa bit of this looks plausible to me; just a couple thoughts about the codegen test.

(Feel free to push back if they won't work or get ugly for some reason.)

View changes since this review

Comment thread tests/codegen-llvm/optimize-closures-inheritance.rs
//! Ensure that `#[optimize]` applied to an outer function is inherited by closures
//! and their coercion shims, unless overridden by an explicit `#[optimize]` on the closure.

//@ compile-flags: -Copt-level=3

@scottmcm scottmcm Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I think you can do this as a -C no-prepopulate-passes test, right? Since we're the ones setting the attributes?

That would mean you'd have no need to handle all the attributes introduced today (or could be introduced in the future) in the CHECK lines, which would hopefully let it be a touch simpler.

(Might even be able to remove the side_effect functions because with no LLVM optimizations going on that shouldn't matter? Or at least only need one of them without also needing -Z merge-functions=disabled.)

Comment thread tests/codegen-llvm/optimize-closures-inheritance.rs
#![crate_type = "lib"]

// CHECK-DAG: define{{.*}}void {{.*}}test_none{{.*}}call_once{{.*}}#[[ATTR_NONE:[0-9]+]]
// CHECK-DAG: define{{.*}}void {{.*}}test_none{{.*}}B{{[0-9]+}}_() {{.*}}#[[ATTR_NONE]]

@scottmcm scottmcm Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly a silly question: what's that B from?

(I get worried about anything too specific in name checks because we've been bitten before by people doing things like %[[FOO]].0 and depending on the exact internal LLVM naming of SRoA results and such. This is probably not that bad, but I'm confused what it is, so...)

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2026
@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scottmcm scottmcm scottmcm requested changes
@fee1-dead fee1-dead fee1-dead requested changes
@wesleywiser wesleywiser wesleywiser approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@scottmcm scottmcm

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@veluca93 @rustbot @fee1-dead @tmandry @wesleywiser @scottmcm @mejrs